Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format Content-Security-Policy header directives #362

Merged
merged 5 commits into from
Mar 18, 2024
Merged

Conversation

dcog989
Copy link
Contributor

@dcog989 dcog989 commented Mar 12, 2024

No description provided.

@LeoColomb LeoColomb linked an issue Mar 12, 2024 that may be closed by this pull request
@LeoColomb LeoColomb changed the title Format Content-Security-Policy header directives #359 Format Content-Security-Policy header directives Mar 12, 2024
Copy link
Member

@LeoColomb LeoColomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR @dcog989.
As you can read in the checks of the PR, the current config is not valid.
Have you tested it on your side?
I believe the following rules apply for multiline expression:

  • The last character on all lines except the final one must be a backslash;
  • The final line must not terminate with a backslash;
  • The Apache comment character (#) cannot be used to comment out a line.
  • You cannot break the flags

Fix incorrect location of comment line.
@dcog989
Copy link
Contributor Author

dcog989 commented Mar 12, 2024

Hi Léo. I saw the errors but the meaning wasn't clear to me. I'm new to github and not familiar with all the tools available - and can't find a way to perform the test done when pull request submitted.

However, I think the reason for failure was the location of comment line:

<IfModule mod_headers.c>
    Header always set Content-Security-Policy "
	# (1) (2) (3) (4) (5) (6) (7)

should be:

<IfModule mod_headers.c>
	# (1) (2) (3) (4) (5) (6) (7)
    Header always set Content-Security-Policy "

I'll attempt to update....

@dcog989 dcog989 requested a review from LeoColomb March 12, 2024 22:36
Copy link
Member

@LeoColomb LeoColomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same review I'm afraid, the current config is not valid.
Have you tested it on your side?
I believe the following rules apply for multiline expression:

  • The last character on all lines except the final one must be a backslash;
  • The final line must not terminate with a backslash;
  • The Apache comment character (#) cannot be used to comment out a line.
  • You cannot break the flags

Test output can be seen in here: https://github.com/h5bp/server-configs-apache/actions/runs/8256582544/job/22646947712?pr=362#step:4:778

@dcog989
Copy link
Contributor Author

dcog989 commented Mar 14, 2024

Hey. Now I'm confused - my re-run shows failure status, but here shows 'All checks have passed'?!!

As you can see, I added backslashes as per spec. Not sure if I have complied with "You cannot break the flags".

@dcog989 dcog989 requested a review from LeoColomb March 14, 2024 09:38
Copy link
Member

@LeoColomb LeoColomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks all right now, thanks @dcog989 😊

@LeoColomb LeoColomb enabled auto-merge (squash) March 18, 2024 10:22
@LeoColomb LeoColomb merged commit aed932a into h5bp:main Mar 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format Content-Security-Policy header directives
2 participants